Skip to content

fix(spec): refuse an absent value on a value-taking view filter operator at authoring time - #19861

Draft
objectstack-fleet[bot] wants to merge 2 commits into
mainfrom
claude/issue-19751-filter-rule-absent-value
Draft

objectstack-fleet[bot] wants to merge 2 commits into
mainfrom
claude/issue-19751-filter-rule-absent-value

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Fixes #19751

Clause-②: no

What changes

checkViewFilterRuleValueShape (the value-shape refinement of ViewFilterRuleSchema, packages/spec/src/ui/view.zod.ts) now refuses a rule with NO value on every operator that takes one. Its scalar arm returned early on value === undefined for every operator, so { field: 'name', operator: 'icontains' } parsed green, while the key's published description says every operator outside in / not_in / between and the four unary operators takes a scalar, and the query path refuses the lowered rule with 400 INVALID_FILTER.

  • The four unary operators (is_empty, is_not_empty, is_null, is_not_null) are answered first and stay valueless, with or without a value.
  • in / not_in / between keep their own arms, which already refused an absent value.
  • The key stays .optional() on the shape; the coupling lives in the refinement, like the other arms.
  • The value .describe() is unchanged (it already declares this contract), so no generated reference page moves.
  • The code comment above the scalar arm, which named an absent value as a carve-out "the query path itself makes", now says the opposite and why. The docblock's "mirrors the query path" list and its runtime-wording section name the new arm.

Refusal text, one issue at the rule's value path:

Filter comparand for operator "icontains" on field "name" is undefined. The rule carries no value, and "icontains" compares the field against one — write the value to compare against, or, if the rule means the field has no value, use an operator that takes none ("is_empty" / "is_not_empty" / "is_null" / "is_not_null"), which reads its direction from its name. This is refused at authoring time because the query path refuses it too (400 INVALID_FILTER).

The leading sentence is the runtime's undefined-comparand sentence ("Filter comparand at PATH is undefined.") with the location named in the view vocabulary: operator and field, the same substitution the list and range arms already make. A view rule has no where path, and the $ spelling in that path is not one a view author can write. The unary operator names in the tail come from the schema's own VIEW_FILTER_VALUELESS_OPERATORS.

Producer reading (step 1): objectui at the pinned .objectui-sha 87af769e9a3ee28ace099fdd653d3ebd79fe82e2

Read with git show SHA:PATH from a local clone at that sha, not from a working tree.

Does the console ever save a value-taking rule with no value? No.

writer file at the pinned sha what happens to a half-filled row
foldFilterGroupToSpecRules, the one fold every view-filter writer shares packages/app-shell/src/views/viewFilterFold.ts a row whose operator takes a value is dropped when isFilterValueComplete(operator, value) is false (if (takesValue && isMissingValue(...)) continue)
isFilterValueComplete packages/components/src/custom/filter-builder.tsx false for value == null (also '', [], a half-filled pair), so an absent value is always incomplete
FilterBuilderField / FilterBuilderWidget: the filter-builder widget that view.form.ts names for filter and page.form.ts for filterBy, plus the per-tab filter editor packages/app-shell/src/views/metadata-admin/widgets.tsx calls the fold on every change; the runtime ViewConfigPanel hosts the same inspector (ViewConfigPanel.tsx, ViewVariantInspector)
list toolbar packages/app-shell/src/views/ObjectView.tsx no automatic write at all (its docblock: "There is deliberately NO persistViewFilter"); explicit saves go through the fold
drill-down "Save as view", foldUrlFilterTriplesToSpecRules packages/app-shell/src/views/ObjectDataPage.tsx ViewFilterRuleSchema.safeParse per rule, refused rules dropped; the URL triples (drillUrlFilters.ts, parseUrlFilterTriples) skip an empty param and always carry a value

One edge, stated rather than hidden: handleViewConfigSave (ObjectView.tsx) persists the config draft whole. A view whose STORED body already carries such a rule (hand-authored, or written by another tool) and is re-saved through the panel without its filter being touched now gets the refusal at save. That view already fails every query today (next table).

Does anything drop a valueless row between storage and the query? No.

layer file reading
console lowering: viewFilterRuleToNode, behind toFilterNode / mergeFilterNodes (plugin-list buildEffectiveFilter, plugin-view ObjectView, ObjectGrid, RelatedList, LineItemsPanel) objectui packages/core/src/utils/filter-converter.ts a rule without value lowers to the 2-tuple [field, operator] and nothing skips it; its own comment records the runtime throwing INVALID_FILTER / 400 for ['name','icontains']
REST lookup-picker route: lowerViewFilterRule this repo, packages/rest/src/view-filter-rule-lowering.ts the same 2-tuple; the module forwards and never drops
query normalizer this repo, packages/metadata-protocol/src/protocol.ts isFilterAST, then parseFilterAST, which throws

Measured on this tree's spec source (4112752): isFilterAST(['and', ['name','equals'], ['status','equals','open']]) is true, and parseFilterAST of it throws INVALID_FILTER / 400, "Filter comparand at where.$and[0].name is undefined". One valueless rule fails the WHOLE view's query, its good rules included.

So no working flow saves or executes this shape, and refusing it at save breaks nothing that works today.

Today's behaviour for the whole class (step 2)

Measured at origin/main 4112752 by script. The operator list is VIEW_FILTER_OPERATORS read at runtime; the unary set was derived by behaviour from the schema's own scalar arm (an array is refused on every non-list, non-range operator outside the private valueless set).

operators ViewFilterRuleSchema, value omitted, before this change parseFilterAST([field, op])
equals, not_equals, contains, not_contains, icontains, starts_with, ends_with, greater_than, less_than, greater_than_or_equal, less_than_or_equal, before, after (13) ACCEPT throws INVALID_FILTER / 400, "Filter comparand at where.name (or where.name.$op) is undefined"
in, not_in refused by the list arm throws, "requires an ARRAY of values"
between refused by the range arm throws, "requires a [min, max] value array"
is_empty, is_not_empty, is_null, is_not_null accept { "$null": true } / { "$null": false }

After this change the 13 are refused. The other rows are unchanged.

ADR-0087 reading (step 4)

  • This narrows a published accept set. The repo's rule for that during the launch window is in the header of scripts/check-changeset-no-major.mjs: the level does not carry breaking-ness, and "the mandatory information carriers for breaking-ness in the meantime are the BREAKING banner the author writes in the changeset body and the ADR-0087 migration-ledger disposition". scripts/check-adr-0087-registration.mjs then requires a disposition on the declared-breaking changeset.
  • The disposition is registered, not not-required. The author has a hand prescription (write the value, switch to a unary operator, or delete an unfinished row), and no-migration-prescription is refused for a body that carries one. None of the other categories fits: the package publishes, no existing entry covers absence, and the surface is a schema, not a runtime interface or a type surface.
  • Precedents: view-filter-rule-scalar-operator-array-refused (the sibling arm of this same check) and filter-preset-ordering-comparand-refused (a shape that never executed usefully) both registered a semantic entry under protocol major 18.
  • Added: packages/spec/src/migrations/entries/semantic/18.view-filter-rule-absent-value-refused.ts. packages/spec/src/migrations/registry.ts was regenerated by pnpm --filter @objectstack/spec gen:migration-registry and not hand-edited; check:migration-registry is green. No D2 conversion: there is no value to infer.
  • check-adr-0087-registration --base origin/main reads the changeset as [BREAKING+clause-②-narrowing] registered view-filter-rule-absent-value-refused (new here).
  • spec-changes.json and docs/protocol-upgrade-guide.md did not move. The protocol-18 step stays inert until the protocol major reaches 18, and check:spec-changes / check:upgrade-guide are green without regeneration.

Changeset (step 7)

.changeset/19751-view-filter-rule-absent-value-refused.md, patch on @objectstack/spec. files[] ships dist and src/**/*.zod.ts, and both carry the refinement. Its summary is the user-visible change: a stored view filter rule with no value on a value-taking operator is now refused at save instead of failing every query. It carries the BREAKING banner, a FROM/TO block, Clause-②: no (narrowing) and the registered disposition marker.

⚠️ Level: patch follows the dispatch and the "WHICH LEVEL" rule in .github/workflows/pr-automation.yml (the level is graded by the act, and a narrowing adds nothing). Both precedents above shipped minor with the same banner, and the check-changeset-no-major.mjs header says "we ship breaking changes as minor". Both levels pass the gates. This is left to the reviewing seat.

The PR's Clause-②: no line is the claim's, copied verbatim. The changeset adds the (narrowing) arm to the same no value, as the sibling precedent's changeset did, so that the ADR-0087 gate reads the direction from a token and not only from the banner.

Fixtures, examples and pins (step 5)

  • An AST scan of every tracked .ts / .tsx / .mts / .js / .mjs / .json outside content/docs/references/ (1,739 files mention operator) found 311 object literals with field plus a string-literal value-taking operator (aliases folded). 20 of them have no value key, and none is a view filter rule in a shipped example or seed:
    • 7 are QA assertions (expectedValue, a different schema) in examples/app-showcase/qa/platform-smoke.test.json;
    • 5 are QA assertions in packages/core/src/qa/runner.test.ts and packages/spec/src/qa/testing.test.ts;
    • 1 is a skill trigger condition in packages/spec/src/ai/skill-trigger-condition-value-shape.test.ts;
    • 4 are a structural walk with no schema in packages/metadata-protocol/src/protocol.graft-normalized-operators.test.ts;
    • 3 are in view-filter-rule-value-shape.test.ts.
      Markdown (.md / .mdx) has no match. No fixture was an authoring mistake, so no fixture was edited.
  • Pins that pinned the removed carve-out and moved with it:
    • packages/spec/src/ui/view-filter-rule-value-shape.test.ts: equals + omitted and greater_than + omitted, from accepted to refused.
    • packages/spec/src/data/filter-icontains-parse-door.test.ts: "ABSENCE is left unjudged" now asserts that absence is refused once, in the absent-value arm's words and never in the conformance table's. ⚠️ This file is outside the claim's declared file surface. It is a view-filter-rule test that lives in src/data/, not beside view.zod.ts, and it had to move with the carve-out it pinned.
  • Carriers named in the docblock (ListView.filter, a tab filter, Page.filterBy, a related-list filter, a lookup picker filter, plus ObjectGridProps.defaultFilters) are all z.array(ViewFilterRuleSchema). The full spec suite is green, and a new pin drives the refusal through ListView.filter at filter.1.value.

Tests

  • New pins, with operator lists derived at runtime: value-taking is VIEW_FILTER_OPERATORS minus the four valueless operators. The valueless set is module-private in view.zod.ts and deliberately not exported, so the test reuses the file's existing transcription, and a new two-way sweep holds that transcription equal to the private set by behaviour: over every operator, an absent value is accepted exactly when the operator is valueless.
  • vitest run --project local on view-filter-rule-value-shape.test.ts and filter-icontains-parse-door.test.ts: 104 passed.
  • Firing control at 325052f, through scripts/ablation-replace.mjs: the anchor if (value === undefined) { was replaced by if (value === undefined) return; followed by if (false) {, which is the base behaviour (an absent value returns before any issue). The anchor went 1 to 0 and the blob b6b2f445 to 8c2839db. Result: 11 new pins red, 57 green. After the restore, the blob equals HEAD and git diff HEAD is empty. A first attempt was refused by the tool before anything ran, because its replacement contained the anchor; nothing was measured on that attempt.
  • Full spec local project at 22a14a1: 522 files passed, 15,420 tests passed. One file skipped by its own stale-dist condition (scripts/root-entry-type-nameability.pin.test.ts); after a rebuild at the same head it ran with OS_EXPECT_ROOT_NAMEABILITY=1: 2 passed.
  • Spec repo project at 22a14a1: 34 files, 587 tests passed.
  • pnpm --filter @objectstack/spec typecheck at 22a14a1: exit 0 (tsc, scripts typecheck, check:test-typecheck OK).

Gates

node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands at 22a14a1 derived 86 commands. All were run and reconciled with --ran: 84 exit 0, 2 NOT MEASURED, 0 unrun. The two NOT MEASURED are check:dual-build-cjs-loads and check:type-check-debt, both exit 3 PREREQUISITE NOT MET because they need the whole-workspace build. They are left to CI. check:generated is 15/15 up to date after a fresh build at that head.

Generated files that moved: packages/spec/src/migrations/registry.ts only, via gen:migration-registry.

Scope held

Acceptance notes

  • The sibling entry view-filter-rule-scalar-operator-array-refused says, in its replacement prose, "An omitted value is still an omitted value". That was true of its own arm; after this change an omitted value on a scalar operator is refused. Both entries sit in the uncut protocol-18 step. The new entry's leading comment names the reversal, and the sibling's text was left as it is (it is outside this card's file surface).
  • checkViewFilterRuleTextComparand's docblock, carve-out 1, says an omitted comparand "is left to whatever judges absence". That stays true: the shape arm now judges it. Not edited.

Generated by Claude Code

…tor at authoring time

The scalar arm of the view filter rule value check returned early on an
absent value for every operator, so a rule such as
{ field: 'name', operator: 'icontains' } parsed green while its published
description declares a scalar for every non-unary operator, and the query
path refuses the lowered [field, operator] node with 400 INVALID_FILTER.
The four unary operators stay valueless; in / not_in / between keep their
own arms. Adds the ADR-0087 semantic entry under protocol major 18 and the
regenerated migration registry.

Claude-Session: https://claude.ai/code/session_013RDBh5DqXd2xnLwvHLgLFr
Co-authored-by: Claude <noreply@anthropic.com>
…ue arm

The pin asserted that an icontains view rule with no value parses. That
carve-out is gone: the value-shape check now refuses a missing value on
every operator that takes one. The pin keeps its own half, that the text
comparand door never judges absence: the refusal arrives once, in the
absent-value arm's words, never in the conformance table's.

Claude-Session: https://claude.ai/code/session_013RDBh5DqXd2xnLwvHLgLFr
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

2 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • 4 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 136 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 8cbc3c0084a3c3c8ef5e59763ab3be96aa1a6f07packageMentionDocs.

Which tree this was computed on

This run read content/docs from 5540949db6c55ce6e901523ee796cc9b7dfa8419 — the merge of head 22a14a10384056741dda7a2d8e2626901bf8fd56 into base 8cbc3c0084a3c3c8ef5e59763ab3be96aa1a6f07, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 5540949db6c55ce6e901523ee796cc9b7dfa8419 && git checkout 5540949db6c55ce6e901523ee796cc9b7dfa8419
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 8cbc3c0084a3c3c8ef5e59763ab3be96aa1a6f07 22a14a10384056741dda7a2d8e2626901bf8fd56 && git checkout -B drift-repro 8cbc3c0084a3c3c8ef5e59763ab3be96aa1a6f07 && git merge --no-ff 22a14a10384056741dda7a2d8e2626901bf8fd56

node scripts/docs-audit/affected-docs.mjs --json 8cbc3c0084a3c3c8ef5e59763ab3be96aa1a6f07

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

1 participant